fix(providers/celery): Migrate conf imports to SDK compatibility layer#60033
fix(providers/celery): Migrate conf imports to SDK compatibility layer#60033ftakelait wants to merge 1 commit intoapache:mainfrom
Conversation
| ARG_BROKER_API = Arg(("-a", "--broker-api"), help="Broker API") | ||
| ARG_FLOWER_HOSTNAME = Arg( | ||
| ("-H", "--hostname"), | ||
| default=conf.get("celery", "FLOWER_HOST"), |
There was a problem hiding this comment.
Why do we need defaults?
|
Still working on it @ftakelait ? are you going to rebase and fix the comments ? |
Hi @potiuk, I am still waiting for @sunank200 to reply about whether I should keep the |
|
I have not seen your question - it's not visible here - I don't think @sunank200 is rather waiting for you - looking at the comments in PR. #60074 (comment) is where we discussed how to fix it - and hopefully it will be fixed there But you can still rebase though and keep on solving conflicts. That would be my recommendation while waiting for mechanism implemented for k8s |
Replace imports of `from airflow.configuration import conf` with `from airflow.providers.common.compat.sdk import conf` for Airflow 3.x compatibility. Add fallback parameters to conf.get() calls that execute at module import time to prevent AirflowConfigException when config keys are not yet loaded. Part of apache#60000 Signed-off-by: Fouzi Takelait <ftakelait@gmail.com>
ftakelait
force-pushed
the
fix/60000-celery-conf-sdk-migration
branch
from
02f5850 to
5c615be
Compare
|
Rebased the PR. I'll wait for the configuration mechanism being developed in #60074, then update this PR to use it instead of individual fallbacks. |
There was a problem hiding this comment.
There are unresolved conflict markers here.
|
Why do we need to add fallbacks to many configs? Did they not work without the fallbacks? |
@uranusjr -> This is manifestation of doing too much and in a wrong sequence during imports. The problem is that currently the sdk config does not have the defaults "hard-coded" fallbacks for celery and kubernetes providers (@amoghrajesh deliberately skipped them when moving conf to task.sdk) - because initialization of executors (that already need those fallbacks) is happening during importing airflow and before provider's manager initializes configuration retrieved from providers. This has been discussed and I proposed to bring it back as a temporary solution before we finish task isolation and will be able to do explicit initialization rather than import-based initialization: #60074 (comment) But maybe there are other proposals? |
4 participants
Summary
Migrates
confimports in the Celery provider fromairflow.configurationtoairflow.providers.common.compat.sdkfor Airflow 3.x SDK compatibility.Changes Made
Import Migration (9 files)
celery_executor.pycelery_executor_utils.py(2 imports)celery_kubernetes_executor.pycelery_command.pydefault_celery.pyFallback Parameters Added
Added
fallbackparameters toconf.get()calls that execute at module import time:celery_executor.pyFLOWER_HOST"0.0.0.0"celery_executor.pyFLOWER_PORT5555celery_executor.pyFLOWER_URL_PREFIX""celery_executor.pyFLOWER_BASIC_AUTH""celery_executor.pyDEFAULT_QUEUE"default"celery_executor.pyworker_concurrency16celery_executor.pySYNC_PARALLELISM0celery_executor.pytask_publish_max_retries3celery_executor_utils.pyoperation_timeout1.0celery_executor_utils.pyCELERY_APP_NAME"airflow.executors.celery_executor"default_celery.pySSL_ACTIVEFalseWhy fallbacks are needed?
The SDK
conf.get()is stricter thanairflow.configuration.conf.get()- it raisesAirflowConfigExceptionwhen a key is not found instead of returningNone. This was causing test collection failures when config keys were accessed at module import time before provider configuration was loaded.Part of #60000